feat: 어드민에서 파견 대학을 관리하도록 #633
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- HOST_UNIVERSITY_HAS_REFERENCES : 파견 대학 삭제 시 해당 대학을 참조하는 UnivApplyInfo가 존재하는 경우
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java`:
- Around line 58-82: Add a DB migration to add a UNIQUE constraint on
host_university.korean_name and update the service to translate DB uniqueness
violations into the existing domain error; specifically, keep
createHostUniversity(...) and validateKoreanNameNotExists(...) but wrap
hostUniversityRepository.save(hostUniversity) in a try/catch for
DataIntegrityViolationException and rethrow or map it to your
HOST_UNIVERSITY_ALREADY_EXISTS error (or appropriate custom exception/response);
also update the HostUniversity entity to reflect uniqueness if you maintain JPA
metadata (add unique=true on koreanName) so the intent is clear in both the
schema and the model.
- Around line 91-115: The updateHostUniversity method currently performs a full
overwrite including nullable fields (homepageUrl, englishCourseUrl,
accommodationUrl, detailsForLocal) which can unintentionally null out existing
data; change the implementation to perform a merge-style update by either (A)
checking each nullable request field in updateHostUniversity and only calling
HostUniversity.update with values present, or (B) modify HostUniversity.update
to treat null inputs as "no-op" for those specific fields so existing values are
preserved; ensure you still validate required fields via
validateKoreanNameNotDuplicated and findCountryByCode/findRegionByCode and
reference updateHostUniversity and HostUniversity.update when applying the
change.
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/admin/university/controller/AdminHostUniversityController.java (2)
47-53: 1. 생성 응답은 201로 맞추는 편이 좋습니다.
- 클라이언트 계약을 명확히 하려면 201 Created(가능하면 Location 헤더)로 응답하는 게 REST 관례입니다.
🔧 제안 변경
- return ResponseEntity.ok(response); + return ResponseEntity.status(201).body(response);
64-69: 1. 삭제 응답은 204가 더 명확합니다.
- 리소스 삭제는 본문 없는 204 No Content가 일반적입니다.
🔧 제안 변경
- return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build();
...in/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java
Show resolved
Hide resolved
...in/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java
Show resolved
Hide resolved
Gyuhyeok99
approved these changes
Feb 2, 2026
Contributor
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
오우.... 깔끔하게 너무 잘 작성해주셨네요.. 로직에는 문제 없는 거 같아서 간단한 궁금증만 남겨두었습니다!
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @GetMapping("/{id}") |
Contributor
There was a problem hiding this comment.
이건 너무 길어져서 그냥 id로 하신건가요? 그동안은 어떤 id인지 명시했던 거 같아서요!
Member
Author
There was a problem hiding this comment.
앗 맞습니다 제가 누락한 부분이라 수정했습니다 !
| private final AdminHostUniversityService adminHostUniversityService; | ||
|
|
||
| @GetMapping | ||
| public ResponseEntity<AdminHostUniversityListResponse> getHostUniversities( |
Contributor
There was a problem hiding this comment.
PageResponse 라는 공통객체가 있는데 혹시 사용안하신 이유가 있을까요? response객체보니 따로
boolean hasNext,
boolean hasPrevious
를 정의하신 거 같아서요!
- id -> host-university-id
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
어드민에서 파견 대학 관련 CRUD 로직을 작성했습니다. 이미지는 별도의 PR에서 작업하겠습니다 !
검색은 기존의 QueryDSL로 작성된 지원 대학 필터 검색을 참고했습니다.
특이 사항
리뷰 요구사항 (선택)